Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support nested memoize (2nd) #5

Merged
merged 5 commits into from
Nov 1, 2020
Merged

Conversation

dai-shi
Copy link
Owner

@dai-shi dai-shi commented Oct 31, 2020

#4 was suboptimal and didn't work for the case I wanted to make work.

Instead of trackMemo, this propagates affected to parent memoize functions even if it's returning a cached value.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Oct 31, 2020

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 046f24f:

Sandbox Source
Vanilla Configuration
Vanilla Typescript Configuration
React Configuration
React Typescript Configuration

@dai-shi
Copy link
Owner Author

dai-shi commented Oct 31, 2020

https://codesandbox.io/s/vanilla-typescript-forked-d83b8?file=/src/index.ts

@theKashey This is what I wanted and looks ok.

// will look again
console.log(
  getItemA({ state: prevState, id: 3 }) === getItemA({ state, id: 3 })
);

@theKashey
Copy link

👍 , this was a little issue for memoize-state, as nested memoization was doing parent one a little unstable - "affected" keys were changing every time.
I was considering something like touchAffected, but in my case, the underlying memoization was reselect based.


During memoize-state development I added a shallBePure helper, which just runs memoization twice to check that function is pure. Just to found that state access is a part of function pureness.

This touched moment is a missing piece. Thank's @dai-shi, I'll make my stuff a little bit touched as well.

@dai-shi
Copy link
Owner Author

dai-shi commented Oct 31, 2020

I was just about to create a redux example, and found unexpected behavior. Maybe a bug... 🤔

@dai-shi
Copy link
Owner Author

dai-shi commented Nov 1, 2020

https://codesandbox.io/s/proxy-memoize-demo-forked-fi5ni?file=/src/App.tsx
I finally made this work. I suppose this is the best we can do with proxy and weakmap combination.

@dai-shi dai-shi merged commit a66cbc2 into master Nov 1, 2020
@dai-shi dai-shi deleted the nested-memoize-with-affected branch November 1, 2020 00:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants